[SYCL] Add sycl::device initial implementation by KseniyaTikhomirova · Pull Request #176972 · llvm/llvm-project

KseniyaTikhomirova · 2026-01-20T17:20:42Z

This is part of the SYCL support upstreaming effort. The relevant RFCs can be found here:

https://discourse.llvm.org/t/rfc-add-full-support-for-the-sycl-programming-model/74080 https://discourse.llvm.org/t/rfc-sycl-runtime-upstreaming/74479

Plan for next PR:

E2E lit configs & test for get_platforms & get_devices impl
context & USM free functions impl

Signed-off-by: Tikhomirova, Kseniya <kseniya.tikhomirova@intel.com>

KseniyaTikhomirova · 2026-01-20T17:21:09Z

libsycl/include/sycl/__impl/info/device.hpp

+// SYCL 2020 A.3. Device information descriptors.
+namespace info {
+
+enum class partition_property : std::uint32_t {


Comparing to intel/llvm I changed the type of partition enums from intptr to uint.
It was aligned with openCL type:
typedef intptr_t cl_device_partition_property;

but I doubt we should keep it the same way.

KseniyaTikhomirova · 2026-01-20T17:21:21Z

libsycl/src/detail/offload/offload_topology.hpp

-using PlatformWithDevStorageType =
-    std::unordered_map<ol_platform_handle_t, std::vector<ol_device_handle_t>>;
+using Platform2DevContainer =
+    std::vector<std::pair<ol_platform_handle_t, ol_device_handle_t>>;


I realized that it is not correct to use map here since rehash may cause change of device order. I have redone this and corresponding code.

KseniyaTikhomirova · 2026-01-20T17:24:03Z

@tahonermann, @dvrogozh, @sergey-semenov, @bader, @againull, @YuriPlyakhin, @Fznamznon

FYI, published for review.

libsycl/include/sycl/__impl/device.hpp

libsycl/include/sycl/__impl/device_selector.hpp

dvrogozh · 2026-01-20T18:24:22Z

libsycl/src/detail/device_impl.cpp

+    return isCPU();
+  case (aspect::gpu):
+    return isGPU();
+  case (aspect::accelerator):


What's the difference between gpu and accelerator?

accelerator == fpga, It is present in spec so I have to cover it, but we have no plan to get it enabled in liboffload and so in libsycl

libsycl/src/detail/offload/offload_topology.cpp

dvrogozh · 2026-01-20T18:35:46Z

libsycl/src/detail/offload/offload_topology.cpp

+    return;
+
+  // MDevices reallocation is prevented to keep correct ranges in MDeviceRange
+  MDevices.reserve(PlatformsAndDev.size());


Hm. Does MDevices supposed to be empty or it might contain something? If the latter shouldn't that be something like MDevices.reserve(MDevices.size() + PlatformsAndDev.size())?

It must be empty. This operation is done once in the first get_platforms call. After that the data is reused and never changes.

libsycl/src/detail/platform_impl.cpp

libsycl/include/sycl/__impl/device.hpp

libsycl/tools/sycl-ls/sycl-ls.cpp

libsycl/src/device_selector.cpp

libsycl/include/sycl/__impl/device_selector.hpp

Signed-off-by: Tikhomirova, Kseniya <kseniya.tikhomirova@intel.com>

KseniyaTikhomirova · 2026-01-21T14:40:30Z

@dvrogozh & @bader thank you for your review. I have replied to or fixed all your comments.

pinging @dm-vodopyanov who agreed to participate in review for upstreaming.

bader

LGTM in general.
Just a few minor nits in comments.

libsycl/include/sycl/__impl/device.hpp

libsycl/include/sycl/__impl/device_selector.hpp

libsycl/src/detail/platform_impl.cpp

libsycl/tools/sycl-ls/sycl-ls.cpp

Signed-off-by: Tikhomirova, Kseniya <kseniya.tikhomirova@intel.com>

dvrogozh

LGTM

libsycl/src/detail/platform_impl.cpp

libsycl/src/device.cpp

libsycl/src/detail/device_impl.hpp

libsycl/src/device_selector.cpp

againull · 2026-01-23T22:32:20Z

libsycl/src/detail/device_impl.hpp

+      std::string Result;
+      Result.resize(ExpectedSize - 1);
+      callAndThrow(olGetDeviceInfo, MOffloadDevice, olInfo, ExpectedSize,
+                   Result.data());


I think this is UB. olGetDeviceInfo can write ExpectedSize bytes into Result.data() but writable size in Result.data() is only ExpectedSize - 1 after resize.

I think it shouldn't since liboffload (same as UR) counts null terminator in the size while std::string doesn't.
Although I do agree that it's confusing. I had to check why I did it this way before answering this question. So it is not obvious and must be commented or updated to the safe version with resize(ExpectedSize). I will do the second.

Sorry, my bad, checked it, and indeed liboffload counts null terminator, and we are allowed to write null terminator at Result.data() + Result.size() (only writing anything besides null terminator is undefined behavior).

So, if we assume that liboffload guarantees that it counts null terminator (they should document it though) then we should return the original version with resize(ExpectedSize - 1).
Or if we want to stay on safer side, we should leave current version but trim after call:

if (!Result.empty() && Result.back() == '\0') Result.pop_back();

Current version is not very good because, for example for "Intel" count() will return 6 and operator== doesn't work as expected by user.

yeah, I haven't thought about operator.
thanks, I returned it back and added comment ae54270
regarding safer side - I think it will be pretty confusing and will mean expecting of flaky behavior of liboffload (that shouldn't happen, if it changes - that is API breaking change) if we add this.

Signed-off-by: Tikhomirova, Kseniya <kseniya.tikhomirova@intel.com>

KseniyaTikhomirova · 2026-01-26T16:38:23Z

pinging @dklochkov-emb who will review instead @dm-vodopyanov

Signed-off-by: Tikhomirova, Kseniya <kseniya.tikhomirova@intel.com>

This is part of the SYCL support upstreaming effort. The relevant RFCs can be found here: https://discourse.llvm.org/t/rfc-add-full-support-for-the-sycl-programming-model/74080 https://discourse.llvm.org/t/rfc-sycl-runtime-upstreaming/74479 Plan for next PR: E2E lit configs & test for get_platforms & get_devices impl context & USM free functions impl --------- Signed-off-by: Tikhomirova, Kseniya <kseniya.tikhomirova@intel.com>

[SYCL] Add sycl::device initial implementation

372e122

Signed-off-by: Tikhomirova, Kseniya <kseniya.tikhomirova@intel.com>

KseniyaTikhomirova commented Jan 20, 2026

View reviewed changes

Fznamznon added the SYCL https://registry.khronos.org/SYCL label Jan 20, 2026

Fznamznon requested review from Fznamznon, YuriPlyakhin, againull, bader and tahonermann and removed request for tahonermann January 20, 2026 17:36

dvrogozh reviewed Jan 20, 2026

View reviewed changes

bader reviewed Jan 20, 2026

View reviewed changes

fix code-review comments

35ee340

Signed-off-by: Tikhomirova, Kseniya <kseniya.tikhomirova@intel.com>

KseniyaTikhomirova requested review from bader and dvrogozh January 21, 2026 14:44

bader approved these changes Jan 21, 2026

View reviewed changes

fix new comments

b3acc5d

Signed-off-by: Tikhomirova, Kseniya <kseniya.tikhomirova@intel.com>

dvrogozh reviewed Jan 23, 2026

View reviewed changes

bader reviewed Jan 23, 2026

View reviewed changes

libsycl/src/detail/platform_impl.cpp Outdated Show resolved Hide resolved

KseniyaTikhomirova mentioned this pull request Jan 23, 2026

[libsycl] Add lit configuration files and basic test #177407

Open

againull reviewed Jan 23, 2026

View reviewed changes

fix new comments

9dc313f

Signed-off-by: Tikhomirova, Kseniya <kseniya.tikhomirova@intel.com>

KseniyaTikhomirova requested review from againull and dvrogozh January 26, 2026 13:31

return string size back

ae54270

Signed-off-by: Tikhomirova, Kseniya <kseniya.tikhomirova@intel.com>

againull approved these changes Feb 2, 2026

View reviewed changes

Merge branch 'main' into add_device_pr

562873d

Merge branch 'main' into add_device_pr

6a30727

bader merged commit debff92 into llvm:main Feb 6, 2026
10 checks passed

Conversation

KseniyaTikhomirova commented Jan 20, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KseniyaTikhomirova commented Jan 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

KseniyaTikhomirova commented Jan 21, 2026

Uh oh!

bader left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dvrogozh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

againull Jan 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KseniyaTikhomirova Feb 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KseniyaTikhomirova commented Jan 26, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

KseniyaTikhomirova commented Jan 20, 2026 •

edited

Loading

againull Jan 28, 2026 •

edited

Loading

KseniyaTikhomirova Feb 2, 2026 •

edited

Loading